-
-
Notifications
You must be signed in to change notification settings - Fork 224
Settings revamp #2035
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Settings revamp #2035
Conversation
- Moved !join to Moderators (!join > :join) - Removed Un/AddFriend commands - Simplified !pnum (Nil players aren't possible anymore on the server) - More simplification/typechecking for inspect - Removed !getgroupinfo for being too expensive for a player command - Reduce !audioplayer command size to one line - !coords is also reduced to one line now - !wait actually uses the Players level
…#2017) * Fix memory leak of player reference caused by logs * Fix typo
|
but why |
|
Benefits of splitting up the settings:
The readme I added also helps people setup the admin as currently people have to figure it out themselves. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why was this turned off? it's not harmful to the place owner
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a discussion in the discord server about setting settings.CreatorPowers default to false and it even possibly removing the setting since if they want Sceleratis to debug the admin they can just rank him. Also some people may consider that setting to be a backdoor.
…privatechat and tts commands (Epix-Incorporated#2033) * Add can user chat checks to alert, chatnotify, mpm, notify, npm, pm, privatechat and tts commands * Remove debug check * Fix lint issue * rename pcall variables * rename pcall variables * Remove debug print * Fix grammar mistake
* Remove additional ExploitGuiDetection settings * Update DefaultSettings.luau
|
I feel like stuff is to spread out, condense some of these into one |
|
Major flaw here aswell. You have no redundancy for everybody who uses the current Settings. This will break EVERY game using Adonis if you don't add redundancy for the old Settings module. You need to fix this issue or this will never be merged. |
WalkerOfBacon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move command aliases, cooldowns, and permissions to their own script instead of splitting them up
You should also support those that still use the old Settings script
|
@kaiserandaxl @WalkerOfBacon the way the module receives settings has not changed so the old settings is automatically supported, the loader merges all the settings into 1 table, it is extremely unlikely someone would insert adonis then use the old loader. |
@WalkerOfBacon What would be a good module name for that? |
Yeah I think that's possible since I can just detect the instance type |
CommandFunctionality or CommandRelated would do (in my opinion) |
Hmm, what do you think of CommandAdjustments too? I'm stuck between choosing CommandFunctionality and CommandAdjustments now |
|
Ah nice so fixing that merge conflict just added more files to the commit, I probably shouldn't of rebased the branch |
I like CommandAdjustments better actually |
yeahh go ahead and re-do this pr lol |
|
Yeah i'm going to do that now and add what everyone suggested to the new PR, for some reason git wouldn't let me fix the merge conflict until I rebased |
|
Continued here: #2049 |
PoF: Adonis loads on my side but reviewers will have to test if Adonis loads on their side too.